POC: gate RayService zero-downtime upgrade with workload slicing - #11264
POC: gate RayService zero-downtime upgrade with workload slicing#11264kevin85421 wants to merge 10 commits into
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
✅ Deploy Preview for kubernetes-sigs-kueue canceled.
|
|
Hi @kevin85421. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| type ClusterUpgradeOptions struct { | ||
| // The capacity of serve requests the upgraded cluster should scale to handle each interval. | ||
| // Defaults to 100%. | ||
| // +kubebuilder:default:=100 |
There was a problem hiding this comment.
ray-project/kuberay#4841 hasn't been merged right now.
0bdc7b6 to
990a514
Compare
| // and the post-upgrade tear-down as a scale-down, without falling back to the | ||
| // non-slice path. | ||
| // | ||
| // POC limitation: when two children share a group name with different PodSpecs |
990a514 to
d405bb1
Compare
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Switch RayService suspend semantics to KubeRay PR kubernetes-sigs#4841's top-level Spec.Suspend (Kueue's stop switch) while keeping the nested RayClusterSpec.Suspend=true as a persistent template gate, so any child RayCluster KubeRay creates -- including the pending one during a zero-downtime upgrade -- is born suspended. Build PodSets from the live child RayClusters (union by group name, sum counts) so the workload's quota reservation reflects active+pending during the upgrade and routes through EnsureWorkloadSlices as a scale-up/scale-down. Keys stay stable across the 1<->2 transition so the slice chain is preserved. Add a Reconcile post-step that unsuspends child RayClusters once the matching workload slice is admitted, with a race-guard that verifies the admitted slice's PodSet counts already cover the union of children's required counts -- prevents prematurely unsuspending the pending child before the upgrade slice is created. Known POC limitations: - Same group name with different PodSpecs across active/pending uses the first child's template, so quota is computed against that template. - MultiKueue adapter does not yet propagate the new suspend semantics. - No webhook validation guarding the persistent RayClusterSpec.Suspend template gate. Vendored rayservice_types.go is synced from a local kuberay checkout with PR kubernetes-sigs#4841 applied; deepcopy is unchanged since Spec.Suspend is a bool value type.
Covers the happy path (1 CPU/2 GiB ClusterQueue gating the upgrade's pending RayCluster) and reproduction recipes for the three known limitations: heterogeneous PodSpec on the same group name, manual tampering with the persistent RayClusterSpec.Suspend gate, and the unfinished MultiKueue adapter.
…g e2e The POC commit changed (*RayService).PodSets and IsSuspended but never updated the unit tests; after the rebase the upstream tests no longer matched the POC's runtime semantics. Unit tests (rayservice_controller_test.go): - TestPodSets: rewrite for the POC's label-based union model. List child RayClusters by KubeRay's association labels and sum PodSet counts across them, instead of upstream's by-name + autoscaling UpdatePodSets model. New cases: bootstrap (no children -> template), single child (reflects the child's live spec), and zero-downtime upgrade (two children -> counts summed: head 1+1=2, group1 2+2=4). Adds a childRayCluster helper and switches the harness to seed multiple labelled children. - TestIsSuspended: assert on the top-level Spec.Suspend (KubeRay kubernetes-sigs#4841) rather than the nested RayClusterSpec.Suspend. E2E test (kuberay_test.go): - Add "Should gate a zero-downtime upgrade's pending RayCluster on queue quota": a 3-CPU ClusterQueue fits one RayCluster (2 CPU) but not the active+pending union (4 CPU). Asserts the pending child is born suspended, the upgrade workload slice reserves both clusters (head count 2) yet stays Pending, the pending child is NOT unsuspended while quota is insufficient, and the gate opens once quota is added. - Find the initial workload by listing rather than by the owner-derived name, since workload slicing adds a slice suffix to the Workload name.
…h merged kuberay#4841 ray-project/kuberay#4841 merged with the same semantics the POC was built against (top-level Spec.Suspend, creation-only nested RayClusterSpec.Suspend, modifyRayCluster preserving the child's Suspend), so no controller changes are needed. - Remove POC-TESTING.md, poc-queues.yaml, ray-service-sample.yaml (manual testing scaffolding, not part of the upstream change). - Align the hand-edited vendored rayservice_types.go with the merged PR: fix the Suspend field comment wording, add the RayServiceResumed condition reason, and drop the unrelated StepSizePercent comment.
… gating e2e Extend the zero-downtime upgrade gating test past the gate-open step: - After promotion, the old RayCluster is deleted (KubeRay's default 60s RayClusterDeletionDelaySeconds) and only the new cluster remains. - The workload slice scales down in place to the single-cluster shape (head=1, 2 pods total), so the ClusterQueue reserves quota for one RayCluster instead of two. Verified locally against a kind cluster running KubeRay master (kuberay#4841 + kubernetes-sigs#4881): 1 Passed | 0 Failed.
3b8d1dc to
7cf656b
Compare
|
/easycla |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kevin85421 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
cc @mimowo KubeRay still doesn't have a release that includes ray-project/kuberay#4841. Should we wait for KubeRay to have a release with the RayService top-level suspend before reviewing/merging this PR? Or can I cut a branch for a KubeRay alpha release, just for Kueue (not for any other users), to unblock this PR? |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
Summary
POC for #11102. Prevents the pending RayCluster created during a zero-downtime RayService upgrade from running before Kueue admits a workload slice that covers its quota demand.
Depends on ray-project/kuberay#4841 (top-level
RayService.Spec.Suspend+ nested template-suspend semantics). The vendoredrayservice_types.gois synced from a local KubeRay checkout with that PR applied.For a step-by-step walkthrough of how the two suspend fields interact across creation, admission, and a zero-downtime upgrade, see this comment on ray-project/kuberay#4841.
Design
RayService.Spec.RayClusterSpec.Suspend=truetemplate gate at creationSuspend()sets nestedSuspend=true;RunWithPodSetsInfo()deliberately leaves it truePodSets()lists live children via theray.io/originated-from-cr-{name,crd}labels (mirrors KubeRay'sRayServiceRayClustersAssociationOptions), unions PodSets by name and sums counts. Same keys across the 1↔2 child transitions soEnsureWorkloadSliceshandles upgrade as scale-up and post-upgrade as scale-downReconcilepost-stepunsuspendAdmittedChildrenpatches each child RayCluster'sSpec.Suspend=false(not the RayService's) once the latest workload slice is admitted; a race-guard checks the slice's PodSet counts cover the current children's required counts before patching, so the pending child stays gated while the new slice is still pendingSuspend()also sets top-levelSpec.Suspend=true; KubeRay deletes all owned resourcesPrerequisites for testing
ElasticJobsViaWorkloadSlices=trueon the Kueue manager.kueue.x-k8s.io/elastic-job: "true"annotation on the RayService.Known limitations
PodSets()keeps the first child's template, so resource-changing upgrades under-/over-account quota. Same-resource upgrades (rayVersion / image / env) are exact.Spec.RayClusterSpec.Suspend, breaking the gate.rayservice_multikueue_adapter.gohasn't been taught the new suspend semantics.Test plan
Automated:
PodSets()across bootstrap / steady-state / zero-downtime-upgrade child topologies (pkg/controller/jobs/rayservice/rayservice_controller_test.go).test/e2e/singlecluster/extended/kuberay_test.go— "Should gate a zero-downtime upgrade's pending RayCluster on queue quota": a 3-CPU ClusterQueue gates the pending RayCluster during an upgrade, releasing quota admits the upgrade slice and unsuspends the pending child, the old RayCluster is then deleted and quota settles back to a single cluster's reservation.Manual (verified locally on kind + KubeRay master with #4841 applied):
AI disclosure: this PR was developed with assistance from Claude Code; all changes were reviewed by the author.